Conversation
|
Thanks for the contribution! Please review the labels and make any necessary changes. |
Summary of ChangesHello @ahcgnoej, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 react-hook-form과 zod를 사용하여 공통 FormField 컴포넌트를 구현합니다. 전반적으로 컴포넌트 구조와 README 문서는 잘 작성되었습니다. 그러나 몇 가지 개선점을 발견했습니다. 가장 중요한 점은 유효성 검사 에러 메시지가 사용자에게 표시되지 않는 버그입니다. 또한, 에러 메시지를 시각적으로 구분할 수 있도록 스타일링을 추가하고, 사용되지 않는 것으로 보이는 Input.tsx 파일을 정리하는 것을 제안합니다. 자세한 내용은 각 파일에 남긴 리뷰 코멘트를 참고해주세요.
| render={({ field, fieldState }) => { | ||
| //todo: 일단 에러메세지 추출만 함 | ||
| const errorMessage = fieldState.error?.message; | ||
| const message = helperText; |
There was a problem hiding this comment.
유효성 검사 에러 메시지가 사용자에게 표시되지 않고 있습니다. errorMessage를 계산했지만, helperText만 message 변수에 할당되어 FormFieldHelper로 전달됩니다. README.md에 명시된 대로 에러가 발생했을 때 에러 메시지를 보여주려면, errorMessage가 있을 때 helperText 대신 이 값을 사용해야 합니다.
| const message = helperText; | |
| const message = errorMessage ?? helperText; |
There was a problem hiding this comment.
에러메세지 추출만 함 확장가능성 보이면 확장하고 에러메세지 안 쓸 거 같으면 에러 판단만 할 예정
| type Props = { | ||
| children: ReactNode; | ||
| }; | ||
|
|
||
| export function FormFieldHelper({ children }: Props) { | ||
| return ( | ||
| <Text | ||
| className="mt-3 text-[11px] text-right" | ||
| style={{ | ||
| color: colorTokens.contentSecondary, | ||
| }} | ||
| > | ||
| {children} | ||
| </Text> | ||
| ); | ||
| } |
There was a problem hiding this comment.
현재 FormFieldHelper는 한 가지 색상만 지원하여 일반 도움말과 에러 메시지를 시각적으로 구분할 수 없습니다. 에러 메시지를 표시할 때 사용자가 명확히 인지할 수 있도록 isError와 같은 prop을 추가하여 텍스트 색상을 동적으로 변경하는 것을 제안합니다. 예를 들어, 에러일 경우 빨간색으로 표시할 수 있습니다.
| type Props = { | |
| children: ReactNode; | |
| }; | |
| export function FormFieldHelper({ children }: Props) { | |
| return ( | |
| <Text | |
| className="mt-3 text-[11px] text-right" | |
| style={{ | |
| color: colorTokens.contentSecondary, | |
| }} | |
| > | |
| {children} | |
| </Text> | |
| ); | |
| } | |
| type Props = { | |
| children: ReactNode; | |
| isError?: boolean; | |
| }; | |
| export function FormFieldHelper({ children, isError }: Props) { | |
| return ( | |
| <Text | |
| className="mt-3 text-[11px] text-right" | |
| style={{ | |
| color: isError ? colorTokens.danger : colorTokens.contentSecondary, | |
| }} | |
| > | |
| {children} | |
| </Text> | |
| ); | |
| } |
chunjaemin
requested review from
chunjaemin
and removed request for
chunjaemin
There was a problem hiding this comment.
텍스트필드 잘 구현해주셨군요 고생하셨습니다!!
다만 현재 존재하지 않는 tokens파일을 쓰셨다 보니까 CI typecheck를 통과하시지 못한 것으로 나오는 것 같아요(채정님 말씀대로 feat/#6브랜치와 merge 되면 문제는 없을 듯 합니다.)
feat/#6 브랜치 먼저 merge한 후, 업데이트 된 develop 브랜치 기준(tokens파일이 존재하는 시점)에서 다시 PR요청 보내주시면 좋을 것 같습니다!
taegeon2
left a comment
taegeon2
left a comment
There was a problem hiding this comment.
FormField
FormFieldInput
등등 FormField 컴포넌트의 역할이 적절히 분리되어있어 재사용성 부분에서 좋은 것 같습니다.!
제미나이 코드 리뷰에서도 나타났는데, 에러 상태에서의 UI 변경..? 도 있으면 좋을거 같은데 추후 이야기해보면 좋을거 같습니다!
4 participants
📝 요약
⚙️ 작업 내용
🔗 관련 이슈
✅ 체크리스트
💬 리뷰어에게
src/shared/ui/FormField/README.md